ecc: fix crash in wc_ecc_import_point_der_ex on invalid format byte#9986
Open
MarkAtwood wants to merge 2 commits intowolfSSL:masterfrom
Open
ecc: fix crash in wc_ecc_import_point_der_ex on invalid format byte#9986MarkAtwood wants to merge 2 commits intowolfSSL:masterfrom
MarkAtwood wants to merge 2 commits intowolfSSL:masterfrom
Conversation
wc_ecc_import_point_der_ex accepted a single byte 0x02 or 0x03 as a valid compressed EC point, treating the missing X coordinate bytes as zeros. This could allow ECDH with a crafted peer public key. Add length validation: compressed points must be exactly 1 + field_element_size bytes. Reject anything shorter or longer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…byte wc_ecc_import_point_der_ex crashed (double-free/SIGABRT) when given a full-length EC point blob with an invalid first byte (0x01, 0x05, 0xFF, etc.). The function fell through to code paths that partially initialized state, then the cleanup path freed already-freed memory. Add early validation of the format byte and fix cleanup paths to prevent double-free on error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a double-free crash in wc_ecc_import_point_der_ex when parsing EC point blobs with an invalid format byte by validating the prefix earlier, before any multi-precision memory operations occur.
Changes:
- Validate
in[0]point format byte upfront and returnASN_PARSE_Eimmediately on invalid values. - Remove the later redundant point type check.
- Add a compressed-point length check when compressed points are enabled.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+9442
to
+9448
| /* validate point format byte before any memory operations */ | ||
| pointType = in[0]; | ||
| if (pointType != ECC_POINT_UNCOMP && | ||
| pointType != ECC_POINT_COMP_EVEN && | ||
| pointType != ECC_POINT_COMP_ODD) { | ||
| return ASN_PARSE_E; | ||
| } |
Comment on lines
9474
to
+9478
| compressed = 1; | ||
| /* compressed points must be exactly 1 + field_element_size bytes */ | ||
| if (inLen != 1 + (word32)ecc_sets[curve_idx].size) { | ||
| err = ECC_BAD_ARG_E; | ||
| } |
| compressed = 1; | ||
| /* compressed points must be exactly 1 + field_element_size bytes */ | ||
| if (inLen != 1 + (word32)ecc_sets[curve_idx].size) { | ||
| err = ECC_BAD_ARG_E; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wc_ecc_import_point_der_excrashes with a double-free (SIGABRT) when given a full-length EC point blob with an invalid first byte (0x01,0x05,0xFF, etc.).For example, passing a 65-byte buffer (the correct size for an uncompressed P-256 point) but with first byte
0x01instead of0x04causes a SIGABRT.Root Cause
The format byte validation (checking for
0x02,0x03, or0x04) was performed aftermp_clear,mp_init_multi, andSAVE_VECTOR_REGISTERShad already executed. When an invalid byte was detected,errwas set toASN_PARSE_E, but execution continued into code paths that partially initialized state (reading data into mp_ints, etc.). On the error cleanup path, already-freed or uninitialized memory was freed again.The sequence:
mp_clear+mp_init_multi+SAVE_VECTOR_REGISTERSexecuteerr = ASN_PARSE_Eif (err == MP_OKAY)but some don't)Reproducer
Also crashes with prefix bytes
0x05,0xFF, or any byte other than0x02/0x03/0x04when the total length matches a valid uncompressed point size.Security Impact
This is a denial-of-service vector. Any code path that passes untrusted EC point data to
wc_ecc_import_point_der_ex(or indirectly viaEC_POINT_oct2pointin the OpenSSL compat layer) can be crashed by an attacker supplying a malformed public key with an invalid format byte. In TLS, the peer's ECDHE public key is parsed through this path.Fix
Moved the format byte validation to before any memory operations (
mp_clear,mp_init_multi,SAVE_VECTOR_REGISTERS). Invalid format bytes now returnASN_PARSE_Eimmediately, before any state is touched. Removed the now-redundant later check.Test Plan
0x04)0x02/0x03)0x00,0x01,0x05,0xFFreturn an error without crashing🤖 Generated with Claude Code